-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[clang-tidy] add modernize-use-constexpr check #146553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang-tidy Author: Julian Schmidt (5chmidti) ChangesThis check finds all functions and variables that can be declared as Fixes #115622 Patch is 95.12 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/146553.diff 9 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index fd7affd22a463..4535df3451c95 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -19,6 +19,7 @@ set_target_properties(genconfusable PROPERTIES FOLDER "Clang Tools Extra/Sourceg
add_clang_library(clangTidyMiscModule STATIC
ConstCorrectnessCheck.cpp
+ ConstexprCheck.cpp
CoroutineHostileRAIICheck.cpp
DefinitionsInHeadersCheck.cpp
ConfusableIdentifierCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp
new file mode 100644
index 0000000000000..270ac1dd0fa48
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp
@@ -0,0 +1,936 @@
+//===--- ConstexprCheck.cpp - clang-tidy ----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ConstexprCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/OperationKinds.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/StmtCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Support/Casting.h"
+#include <cstddef>
+#include <functional>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+namespace {
+AST_MATCHER(FunctionDecl, locationPermitsConstexpr) {
+ const bool IsInMainFile =
+ Finder->getASTContext().getSourceManager().isInMainFile(
+ Node.getLocation());
+
+ if (IsInMainFile && Node.hasExternalFormalLinkage())
+ return false;
+ if (!IsInMainFile && !Node.isInlined())
+ return false;
+
+ return true;
+}
+
+AST_MATCHER(Expr, isCXX11ConstantExpr) {
+ return !Node.isValueDependent() &&
+ Node.isCXX11ConstantExpr(Finder->getASTContext());
+}
+
+AST_MATCHER(DeclaratorDecl, isInMacro) {
+ const SourceRange R =
+ SourceRange(Node.getInnerLocStart(), Node.getLocation());
+
+ return Node.getLocation().isMacroID() || Node.getEndLoc().isMacroID() ||
+ utils::rangeContainsMacroExpansion(
+ R, &Finder->getASTContext().getSourceManager()) ||
+ utils::rangeIsEntirelyWithinMacroArgument(
+ R, &Finder->getASTContext().getSourceManager());
+}
+
+AST_MATCHER(Decl, hasNoRedecl) {
+ // There is always the actual declaration
+ return !Node.redecls().empty() &&
+ std::next(Node.redecls_begin()) == Node.redecls_end();
+}
+
+AST_MATCHER(Decl, allRedeclsInSameFile) {
+ const SourceManager &SM = Finder->getASTContext().getSourceManager();
+ const SourceLocation L = Node.getLocation();
+ for (const Decl *ReDecl : Node.redecls()) {
+ if (!SM.isWrittenInSameFile(L, ReDecl->getLocation()))
+ return false;
+ }
+ return true;
+}
+
+AST_MATCHER(FunctionDecl, isConstexprSpecified) {
+ return Node.isConstexprSpecified();
+}
+
+bool satisfiesConstructorPropertiesUntil20(const CXXConstructorDecl *Ctor,
+ ASTContext &Ctx) {
+ const CXXRecordDecl *Rec = Ctor->getParent();
+ llvm::SmallPtrSet<const RecordDecl *, 8> Bases{};
+ for (const CXXBaseSpecifier Base : Rec->bases()) {
+ Bases.insert(Base.getType()->getAsRecordDecl());
+ }
+ llvm::SmallPtrSet<const FieldDecl *, 8> Fields{Rec->field_begin(),
+ Rec->field_end()};
+ llvm::SmallPtrSet<const FieldDecl *, 4> Indirects{};
+
+ for (const CXXCtorInitializer *const Init : Ctor->inits()) {
+ const Type *InitType = Init->getBaseClass();
+ if (InitType && InitType->isRecordType()) {
+ const auto *ConstructingInit =
+ llvm::dyn_cast<CXXConstructExpr>(Init->getInit());
+ if (ConstructingInit &&
+ !ConstructingInit->getConstructor()->isConstexprSpecified())
+ return false;
+ }
+
+ if (Init->isBaseInitializer()) {
+ Bases.erase(Init->getBaseClass()->getAsRecordDecl());
+ continue;
+ }
+
+ if (Init->isMemberInitializer()) {
+ const FieldDecl *Field = Init->getMember();
+
+ if (Field->isAnonymousStructOrUnion())
+ Indirects.insert(Field);
+
+ Fields.erase(Field);
+ continue;
+ }
+ }
+
+ for (const auto &Match :
+ match(cxxRecordDecl(forEach(indirectFieldDecl().bind("indirect"))), *Rec,
+ Ctx)) {
+ const auto *IField = Match.getNodeAs<IndirectFieldDecl>("indirect");
+
+ size_t NumInitializations = false;
+ for (const NamedDecl *ND : IField->chain())
+ NumInitializations += Indirects.erase(llvm::dyn_cast<FieldDecl>(ND));
+
+ if (NumInitializations != 1)
+ return false;
+
+ for (const NamedDecl *ND : IField->chain())
+ Fields.erase(llvm::dyn_cast<FieldDecl>(ND));
+ }
+
+ if (!Fields.empty())
+ return false;
+
+ return true;
+}
+
+const Type *unwrapPointee(const Type *T) {
+ if (!T->isPointerOrReferenceType())
+ return T;
+
+ while (T && T->isPointerOrReferenceType()) {
+ if (T->isReferenceType()) {
+ const QualType QType = T->getPointeeType();
+ if (!QType.isNull())
+ T = QType.getTypePtr();
+ } else
+ T = T->getPointeeOrArrayElementType();
+ }
+
+ return T;
+}
+
+bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ const bool ConservativeLiteralType);
+
+bool isLiteralType(const Type *T, const ASTContext &Ctx,
+ const bool ConservativeLiteralType) {
+ if (!T)
+ return false;
+
+ if (!T->isLiteralType(Ctx))
+ return false;
+
+ if (!ConservativeLiteralType)
+ return T->isLiteralType(Ctx) && !T->isVoidType();
+
+ if (T->isIncompleteType() || T->isIncompleteArrayType())
+ return false;
+
+ T = unwrapPointee(T);
+ if (!T)
+ return false;
+
+ assert(!T->isPointerOrReferenceType());
+
+ if (T->isIncompleteType() || T->isIncompleteArrayType())
+ return false;
+
+ if (T->isLiteralType(Ctx))
+ return true;
+
+ if (const auto *Rec = T->getAsCXXRecordDecl()) {
+ if (llvm::any_of(Rec->ctors(), [](const CXXConstructorDecl *Ctor) {
+ return !Ctor->isCopyOrMoveConstructor() &&
+ Ctor->isConstexprSpecified();
+ }))
+ return false;
+
+ for (const CXXBaseSpecifier Base : Rec->bases()) {
+ if (!isLiteralType(Base.getType(), Ctx, ConservativeLiteralType))
+ return false;
+ }
+ }
+
+ if (const Type *ArrayElementType = T->getArrayElementTypeNoTypeQual())
+ return isLiteralType(ArrayElementType, Ctx, ConservativeLiteralType);
+
+ return false;
+}
+
+bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ const bool ConservativeLiteralType) {
+ return isLiteralType(QT.getTypePtr(), Ctx, ConservativeLiteralType);
+}
+
+bool satisfiesProperties11(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified()) {
+ return true;
+ }
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (Method &&
+ (Method->isVirtual() ||
+ !match(cxxMethodDecl(hasBody(cxxTryStmt())), *Method, Ctx).empty()))
+ return false;
+
+ if (const auto *Ctor = llvm::dyn_cast<CXXConstructorDecl>(FDecl);
+ Ctor && (!satisfiesConstructorPropertiesUntil20(Ctor, Ctx) ||
+ llvm::any_of(Ctor->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) {
+ return Base.isVirtual();
+ })))
+ return false;
+
+ if (const auto *Dtor = llvm::dyn_cast<CXXDestructorDecl>(FDecl);
+ Dtor && !Dtor->isTrivial())
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor11 : public clang::RecursiveASTVisitor<Visitor11> {
+ public:
+ using Base = clang::RecursiveASTVisitor<Visitor11>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor11(ASTContext &Ctx, bool ConservativeLiteralType)
+ : Ctx(Ctx), ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool WalkUpFromNullStmt(NullStmt *) {
+ Possible = false;
+ return true;
+ }
+ bool WalkUpFromDeclStmt(DeclStmt *DS) {
+ for (const Decl *D : DS->decls())
+ if (!llvm::isa<StaticAssertDecl, TypedefNameDecl, UsingDecl,
+ UsingDirectiveDecl>(D)) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromExpr(Expr *) { return true; }
+ bool WalkUpFromCompoundStmt(CompoundStmt *S) {
+ for (const DynTypedNode &Node : Ctx.getParents(*S))
+ if (Node.get<FunctionDecl>() != nullptr)
+ return true;
+
+ Possible = false;
+ return false;
+ }
+ bool WalkUpFromStmt(Stmt *) {
+ Possible = false;
+ return false;
+ }
+
+ bool WalkUpFromReturnStmt(ReturnStmt *) {
+ ++NumReturns;
+ if (NumReturns != 1U) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromCastExpr(CastExpr *CE) {
+ if (llvm::is_contained(
+ {
+ CK_LValueBitCast,
+ CK_IntegralToPointer,
+ CK_PointerToIntegral,
+ },
+ CE->getCastKind())) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXDynamicCastExpr(CXXDynamicCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseCXXReinterpretCastExpr(CXXReinterpretCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseType(QualType QT) {
+ if (QT.isNull())
+ return true;
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType)) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseType(QT);
+ }
+
+ bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) {
+ if (const auto *Ctor = CE->getConstructor();
+ Ctor && !Ctor->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+
+ return true;
+ }
+ bool WalkUpFromCallExpr(CallExpr *CE) {
+ if (const auto *FDecl =
+ llvm::dyn_cast_if_present<FunctionDecl>(CE->getCalleeDecl());
+ FDecl && !FDecl->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ bool Possible = true;
+ size_t NumReturns = 0;
+ };
+
+ Visitor11 V{Ctx, ConservativeLiteralType};
+ V.TraverseDecl(const_cast<FunctionDecl *>(FDecl));
+ if (!V.Possible)
+ return false;
+
+ return true;
+}
+
+// The only difference between C++14 and C++17 is that `constexpr` lambdas
+// can be used in C++17.
+bool satisfiesProperties1417(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified())
+ return true;
+
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (Method && Method->isVirtual())
+ return false;
+
+ if (llvm::isa<CXXConstructorDecl>(FDecl) &&
+ llvm::any_of(
+ Method->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) { return Base.isVirtual(); }))
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor14 : public clang::RecursiveASTVisitor<Visitor14> {
+ public:
+ using Base = clang::RecursiveASTVisitor<Visitor14>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor14(bool CXX17, ASTContext &Ctx, bool ConservativeLiteralType,
+ bool AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ : CXX17(CXX17), Ctx(Ctx),
+ ConservativeLiteralType(ConservativeLiteralType),
+ AddConstexprToMethodOfClassWithoutConstexprConstructor(
+ AddConstexprToMethodOfClassWithoutConstexprConstructor) {}
+
+ bool TraverseGotoStmt(GotoStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseLabelStmt(LabelStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXTryStmt(CXXTryStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseGCCAsmStmt(GCCAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseMSAsmStmt(MSAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseDecompositionDecl(DecompositionDecl * /*DD*/) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseVarDecl(VarDecl *VD) {
+ const auto StorageDur = VD->getStorageDuration();
+ Possible = VD->hasInit() &&
+ isLiteralType(VD->getType(), VD->getASTContext(),
+ ConservativeLiteralType) &&
+ (StorageDur != StorageDuration::SD_Static &&
+ StorageDur != StorageDuration::SD_Thread);
+ return Possible && Base::TraverseVarDecl(VD);
+ }
+ bool TraverseLambdaExpr(LambdaExpr *LE) {
+ if (CXX17) {
+ Possible = satisfiesProperties1417(
+ LE->getCallOperator(), Ctx, ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor);
+ return Possible;
+ }
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseDeclRefExpr(DeclRefExpr *DRef) {
+ if (const auto *D = llvm::dyn_cast_if_present<VarDecl>(DRef->getDecl());
+ D && !D->isLocalVarDeclOrParm() && D->hasGlobalStorage()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromCastExpr(CastExpr *CE) {
+ if (llvm::is_contained(
+ {
+ CK_LValueBitCast,
+ CK_IntegralToPointer,
+ CK_PointerToIntegral,
+ },
+ CE->getCastKind())) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXDynamicCastExpr(CXXDynamicCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseCXXReinterpretCastExpr(CXXReinterpretCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ const bool CXX17;
+ bool Possible = true;
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor;
+ };
+
+ Visitor14 V{Ctx.getLangOpts().CPlusPlus17 != 0, Ctx, ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor};
+ V.TraverseDecl(const_cast<FunctionDecl *>(FDecl));
+ if (!V.Possible)
+ return false;
+
+ if (const auto *Ctor = llvm::dyn_cast<CXXConstructorDecl>(FDecl);
+ Ctor && !satisfiesConstructorPropertiesUntil20(Ctor, Ctx))
+ return false;
+
+ if (const auto *Dtor = llvm::dyn_cast<CXXDestructorDecl>(FDecl);
+ Dtor && !Dtor->isTrivial())
+ return false;
+
+ class BodyVisitor : public clang::RecursiveASTVisitor<BodyVisitor> {
+ public:
+ using Base = clang::RecursiveASTVisitor<BodyVisitor>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ explicit BodyVisitor(ASTContext &Ctx, bool ConservativeLiteralType)
+ : Ctx(Ctx), ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool TraverseType(QualType QT) {
+ if (QT.isNull())
+ return true;
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType)) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseType(QT);
+ }
+
+ bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) {
+ if (const auto *Ctor = CE->getConstructor();
+ Ctor && !Ctor->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+
+ return true;
+ }
+ bool WalkUpFromCallExpr(CallExpr *CE) {
+ if (const auto *FDecl =
+ llvm::dyn_cast_if_present<FunctionDecl>(CE->getCalleeDecl());
+ FDecl && !FDecl->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ bool Possible = true;
+ };
+
+ if (FDecl->hasBody() && ConservativeLiteralType) {
+ BodyVisitor Visitor(Ctx, ConservativeLiteralType);
+ Visitor.TraverseStmt(FDecl->getBody());
+ if (!Visitor.Possible)
+ return false;
+ }
+ return true;
+}
+
+bool satisfiesProperties20(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified()) {
+ return true;
+ }
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (FDecl->hasBody() && llvm::isa<CoroutineBodyStmt>(FDecl->getBody()))
+ return false;
+
+ if ((llvm::isa<CXXConstructorDecl>(FDecl) ||
+ llvm::isa<CXXDestructorDecl>(FDecl)) &&
+ llvm::any_of(
+ Method->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) { return Base.isVirtual(); }))
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor20 : public clang::RecursiveASTVisitor<Visitor20> {
+ public:
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor20(bool ConservativeLiteralType)
+ : ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool TraverseGotoStmt(GotoStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseLabelStmt(LabelStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXTryStmt(CXXTryStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseGCCAsmStmt(GCCAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseMSAsmStmt(MSAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseDecompositionDecl(DecompositionDecl * /*DD*/) {
+ Possible = f...
[truncated]
|
@llvm/pr-subscribers-clang-tools-extra Author: Julian Schmidt (5chmidti) ChangesThis check finds all functions and variables that can be declared as Fixes #115622 Patch is 95.12 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/146553.diff 9 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index fd7affd22a463..4535df3451c95 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -19,6 +19,7 @@ set_target_properties(genconfusable PROPERTIES FOLDER "Clang Tools Extra/Sourceg
add_clang_library(clangTidyMiscModule STATIC
ConstCorrectnessCheck.cpp
+ ConstexprCheck.cpp
CoroutineHostileRAIICheck.cpp
DefinitionsInHeadersCheck.cpp
ConfusableIdentifierCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp
new file mode 100644
index 0000000000000..270ac1dd0fa48
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp
@@ -0,0 +1,936 @@
+//===--- ConstexprCheck.cpp - clang-tidy ----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ConstexprCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/OperationKinds.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/StmtCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Support/Casting.h"
+#include <cstddef>
+#include <functional>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+namespace {
+AST_MATCHER(FunctionDecl, locationPermitsConstexpr) {
+ const bool IsInMainFile =
+ Finder->getASTContext().getSourceManager().isInMainFile(
+ Node.getLocation());
+
+ if (IsInMainFile && Node.hasExternalFormalLinkage())
+ return false;
+ if (!IsInMainFile && !Node.isInlined())
+ return false;
+
+ return true;
+}
+
+AST_MATCHER(Expr, isCXX11ConstantExpr) {
+ return !Node.isValueDependent() &&
+ Node.isCXX11ConstantExpr(Finder->getASTContext());
+}
+
+AST_MATCHER(DeclaratorDecl, isInMacro) {
+ const SourceRange R =
+ SourceRange(Node.getInnerLocStart(), Node.getLocation());
+
+ return Node.getLocation().isMacroID() || Node.getEndLoc().isMacroID() ||
+ utils::rangeContainsMacroExpansion(
+ R, &Finder->getASTContext().getSourceManager()) ||
+ utils::rangeIsEntirelyWithinMacroArgument(
+ R, &Finder->getASTContext().getSourceManager());
+}
+
+AST_MATCHER(Decl, hasNoRedecl) {
+ // There is always the actual declaration
+ return !Node.redecls().empty() &&
+ std::next(Node.redecls_begin()) == Node.redecls_end();
+}
+
+AST_MATCHER(Decl, allRedeclsInSameFile) {
+ const SourceManager &SM = Finder->getASTContext().getSourceManager();
+ const SourceLocation L = Node.getLocation();
+ for (const Decl *ReDecl : Node.redecls()) {
+ if (!SM.isWrittenInSameFile(L, ReDecl->getLocation()))
+ return false;
+ }
+ return true;
+}
+
+AST_MATCHER(FunctionDecl, isConstexprSpecified) {
+ return Node.isConstexprSpecified();
+}
+
+bool satisfiesConstructorPropertiesUntil20(const CXXConstructorDecl *Ctor,
+ ASTContext &Ctx) {
+ const CXXRecordDecl *Rec = Ctor->getParent();
+ llvm::SmallPtrSet<const RecordDecl *, 8> Bases{};
+ for (const CXXBaseSpecifier Base : Rec->bases()) {
+ Bases.insert(Base.getType()->getAsRecordDecl());
+ }
+ llvm::SmallPtrSet<const FieldDecl *, 8> Fields{Rec->field_begin(),
+ Rec->field_end()};
+ llvm::SmallPtrSet<const FieldDecl *, 4> Indirects{};
+
+ for (const CXXCtorInitializer *const Init : Ctor->inits()) {
+ const Type *InitType = Init->getBaseClass();
+ if (InitType && InitType->isRecordType()) {
+ const auto *ConstructingInit =
+ llvm::dyn_cast<CXXConstructExpr>(Init->getInit());
+ if (ConstructingInit &&
+ !ConstructingInit->getConstructor()->isConstexprSpecified())
+ return false;
+ }
+
+ if (Init->isBaseInitializer()) {
+ Bases.erase(Init->getBaseClass()->getAsRecordDecl());
+ continue;
+ }
+
+ if (Init->isMemberInitializer()) {
+ const FieldDecl *Field = Init->getMember();
+
+ if (Field->isAnonymousStructOrUnion())
+ Indirects.insert(Field);
+
+ Fields.erase(Field);
+ continue;
+ }
+ }
+
+ for (const auto &Match :
+ match(cxxRecordDecl(forEach(indirectFieldDecl().bind("indirect"))), *Rec,
+ Ctx)) {
+ const auto *IField = Match.getNodeAs<IndirectFieldDecl>("indirect");
+
+ size_t NumInitializations = false;
+ for (const NamedDecl *ND : IField->chain())
+ NumInitializations += Indirects.erase(llvm::dyn_cast<FieldDecl>(ND));
+
+ if (NumInitializations != 1)
+ return false;
+
+ for (const NamedDecl *ND : IField->chain())
+ Fields.erase(llvm::dyn_cast<FieldDecl>(ND));
+ }
+
+ if (!Fields.empty())
+ return false;
+
+ return true;
+}
+
+const Type *unwrapPointee(const Type *T) {
+ if (!T->isPointerOrReferenceType())
+ return T;
+
+ while (T && T->isPointerOrReferenceType()) {
+ if (T->isReferenceType()) {
+ const QualType QType = T->getPointeeType();
+ if (!QType.isNull())
+ T = QType.getTypePtr();
+ } else
+ T = T->getPointeeOrArrayElementType();
+ }
+
+ return T;
+}
+
+bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ const bool ConservativeLiteralType);
+
+bool isLiteralType(const Type *T, const ASTContext &Ctx,
+ const bool ConservativeLiteralType) {
+ if (!T)
+ return false;
+
+ if (!T->isLiteralType(Ctx))
+ return false;
+
+ if (!ConservativeLiteralType)
+ return T->isLiteralType(Ctx) && !T->isVoidType();
+
+ if (T->isIncompleteType() || T->isIncompleteArrayType())
+ return false;
+
+ T = unwrapPointee(T);
+ if (!T)
+ return false;
+
+ assert(!T->isPointerOrReferenceType());
+
+ if (T->isIncompleteType() || T->isIncompleteArrayType())
+ return false;
+
+ if (T->isLiteralType(Ctx))
+ return true;
+
+ if (const auto *Rec = T->getAsCXXRecordDecl()) {
+ if (llvm::any_of(Rec->ctors(), [](const CXXConstructorDecl *Ctor) {
+ return !Ctor->isCopyOrMoveConstructor() &&
+ Ctor->isConstexprSpecified();
+ }))
+ return false;
+
+ for (const CXXBaseSpecifier Base : Rec->bases()) {
+ if (!isLiteralType(Base.getType(), Ctx, ConservativeLiteralType))
+ return false;
+ }
+ }
+
+ if (const Type *ArrayElementType = T->getArrayElementTypeNoTypeQual())
+ return isLiteralType(ArrayElementType, Ctx, ConservativeLiteralType);
+
+ return false;
+}
+
+bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ const bool ConservativeLiteralType) {
+ return isLiteralType(QT.getTypePtr(), Ctx, ConservativeLiteralType);
+}
+
+bool satisfiesProperties11(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified()) {
+ return true;
+ }
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (Method &&
+ (Method->isVirtual() ||
+ !match(cxxMethodDecl(hasBody(cxxTryStmt())), *Method, Ctx).empty()))
+ return false;
+
+ if (const auto *Ctor = llvm::dyn_cast<CXXConstructorDecl>(FDecl);
+ Ctor && (!satisfiesConstructorPropertiesUntil20(Ctor, Ctx) ||
+ llvm::any_of(Ctor->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) {
+ return Base.isVirtual();
+ })))
+ return false;
+
+ if (const auto *Dtor = llvm::dyn_cast<CXXDestructorDecl>(FDecl);
+ Dtor && !Dtor->isTrivial())
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor11 : public clang::RecursiveASTVisitor<Visitor11> {
+ public:
+ using Base = clang::RecursiveASTVisitor<Visitor11>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor11(ASTContext &Ctx, bool ConservativeLiteralType)
+ : Ctx(Ctx), ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool WalkUpFromNullStmt(NullStmt *) {
+ Possible = false;
+ return true;
+ }
+ bool WalkUpFromDeclStmt(DeclStmt *DS) {
+ for (const Decl *D : DS->decls())
+ if (!llvm::isa<StaticAssertDecl, TypedefNameDecl, UsingDecl,
+ UsingDirectiveDecl>(D)) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromExpr(Expr *) { return true; }
+ bool WalkUpFromCompoundStmt(CompoundStmt *S) {
+ for (const DynTypedNode &Node : Ctx.getParents(*S))
+ if (Node.get<FunctionDecl>() != nullptr)
+ return true;
+
+ Possible = false;
+ return false;
+ }
+ bool WalkUpFromStmt(Stmt *) {
+ Possible = false;
+ return false;
+ }
+
+ bool WalkUpFromReturnStmt(ReturnStmt *) {
+ ++NumReturns;
+ if (NumReturns != 1U) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromCastExpr(CastExpr *CE) {
+ if (llvm::is_contained(
+ {
+ CK_LValueBitCast,
+ CK_IntegralToPointer,
+ CK_PointerToIntegral,
+ },
+ CE->getCastKind())) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXDynamicCastExpr(CXXDynamicCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseCXXReinterpretCastExpr(CXXReinterpretCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseType(QualType QT) {
+ if (QT.isNull())
+ return true;
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType)) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseType(QT);
+ }
+
+ bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) {
+ if (const auto *Ctor = CE->getConstructor();
+ Ctor && !Ctor->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+
+ return true;
+ }
+ bool WalkUpFromCallExpr(CallExpr *CE) {
+ if (const auto *FDecl =
+ llvm::dyn_cast_if_present<FunctionDecl>(CE->getCalleeDecl());
+ FDecl && !FDecl->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ bool Possible = true;
+ size_t NumReturns = 0;
+ };
+
+ Visitor11 V{Ctx, ConservativeLiteralType};
+ V.TraverseDecl(const_cast<FunctionDecl *>(FDecl));
+ if (!V.Possible)
+ return false;
+
+ return true;
+}
+
+// The only difference between C++14 and C++17 is that `constexpr` lambdas
+// can be used in C++17.
+bool satisfiesProperties1417(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified())
+ return true;
+
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (Method && Method->isVirtual())
+ return false;
+
+ if (llvm::isa<CXXConstructorDecl>(FDecl) &&
+ llvm::any_of(
+ Method->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) { return Base.isVirtual(); }))
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor14 : public clang::RecursiveASTVisitor<Visitor14> {
+ public:
+ using Base = clang::RecursiveASTVisitor<Visitor14>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor14(bool CXX17, ASTContext &Ctx, bool ConservativeLiteralType,
+ bool AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ : CXX17(CXX17), Ctx(Ctx),
+ ConservativeLiteralType(ConservativeLiteralType),
+ AddConstexprToMethodOfClassWithoutConstexprConstructor(
+ AddConstexprToMethodOfClassWithoutConstexprConstructor) {}
+
+ bool TraverseGotoStmt(GotoStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseLabelStmt(LabelStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXTryStmt(CXXTryStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseGCCAsmStmt(GCCAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseMSAsmStmt(MSAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseDecompositionDecl(DecompositionDecl * /*DD*/) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseVarDecl(VarDecl *VD) {
+ const auto StorageDur = VD->getStorageDuration();
+ Possible = VD->hasInit() &&
+ isLiteralType(VD->getType(), VD->getASTContext(),
+ ConservativeLiteralType) &&
+ (StorageDur != StorageDuration::SD_Static &&
+ StorageDur != StorageDuration::SD_Thread);
+ return Possible && Base::TraverseVarDecl(VD);
+ }
+ bool TraverseLambdaExpr(LambdaExpr *LE) {
+ if (CXX17) {
+ Possible = satisfiesProperties1417(
+ LE->getCallOperator(), Ctx, ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor);
+ return Possible;
+ }
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseDeclRefExpr(DeclRefExpr *DRef) {
+ if (const auto *D = llvm::dyn_cast_if_present<VarDecl>(DRef->getDecl());
+ D && !D->isLocalVarDeclOrParm() && D->hasGlobalStorage()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromCastExpr(CastExpr *CE) {
+ if (llvm::is_contained(
+ {
+ CK_LValueBitCast,
+ CK_IntegralToPointer,
+ CK_PointerToIntegral,
+ },
+ CE->getCastKind())) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXDynamicCastExpr(CXXDynamicCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseCXXReinterpretCastExpr(CXXReinterpretCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ const bool CXX17;
+ bool Possible = true;
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor;
+ };
+
+ Visitor14 V{Ctx.getLangOpts().CPlusPlus17 != 0, Ctx, ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor};
+ V.TraverseDecl(const_cast<FunctionDecl *>(FDecl));
+ if (!V.Possible)
+ return false;
+
+ if (const auto *Ctor = llvm::dyn_cast<CXXConstructorDecl>(FDecl);
+ Ctor && !satisfiesConstructorPropertiesUntil20(Ctor, Ctx))
+ return false;
+
+ if (const auto *Dtor = llvm::dyn_cast<CXXDestructorDecl>(FDecl);
+ Dtor && !Dtor->isTrivial())
+ return false;
+
+ class BodyVisitor : public clang::RecursiveASTVisitor<BodyVisitor> {
+ public:
+ using Base = clang::RecursiveASTVisitor<BodyVisitor>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ explicit BodyVisitor(ASTContext &Ctx, bool ConservativeLiteralType)
+ : Ctx(Ctx), ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool TraverseType(QualType QT) {
+ if (QT.isNull())
+ return true;
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType)) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseType(QT);
+ }
+
+ bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) {
+ if (const auto *Ctor = CE->getConstructor();
+ Ctor && !Ctor->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+
+ return true;
+ }
+ bool WalkUpFromCallExpr(CallExpr *CE) {
+ if (const auto *FDecl =
+ llvm::dyn_cast_if_present<FunctionDecl>(CE->getCalleeDecl());
+ FDecl && !FDecl->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ bool Possible = true;
+ };
+
+ if (FDecl->hasBody() && ConservativeLiteralType) {
+ BodyVisitor Visitor(Ctx, ConservativeLiteralType);
+ Visitor.TraverseStmt(FDecl->getBody());
+ if (!Visitor.Possible)
+ return false;
+ }
+ return true;
+}
+
+bool satisfiesProperties20(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified()) {
+ return true;
+ }
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (FDecl->hasBody() && llvm::isa<CoroutineBodyStmt>(FDecl->getBody()))
+ return false;
+
+ if ((llvm::isa<CXXConstructorDecl>(FDecl) ||
+ llvm::isa<CXXDestructorDecl>(FDecl)) &&
+ llvm::any_of(
+ Method->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) { return Base.isVirtual(); }))
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor20 : public clang::RecursiveASTVisitor<Visitor20> {
+ public:
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor20(bool ConservativeLiteralType)
+ : ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool TraverseGotoStmt(GotoStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseLabelStmt(LabelStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXTryStmt(CXXTryStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseGCCAsmStmt(GCCAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseMSAsmStmt(MSAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseDecompositionDecl(DecompositionDecl * /*DD*/) {
+ Possible = f...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
35a081f
to
2a50c3e
Compare
misc-use-constexpr? :) |
I ran this check on all of Some perf info from running the checks enabled in LLVM + this check: Running on:
Running on
|
modernize-use-constexpr? |
2a50c3e
to
8d35424
Compare
This check finds all functions and variables that can be declared as `constexpr`, using the specified standard version to check if the requirements are met. Fixes llvm#115622
8d35424
to
16d5a3f
Compare
Hm. It is indeed also a modernization... +- on which to pick. Let's see what others think.
while the misc category has constexpr int div(int a, int b) { return a / b; }
int main() {
constexpr auto res = div(10,0);
return 0;
} , or potential performance improvements. |
|
I'm +1 on the |
I'm ok with modernize-use-constexpr as well! |
I tried with ITK and the resulting transformation did not compile. Many changes were of this form: - const double Max = 1.0 - Min;
+ constexpr double Max = 1.0 - Min; Which is great, though notice the double space after Other changes were like this: - const auto check = [](const auto & ptr) { EXPECT_THROW(itk::Deref(ptr), itk::DerefError); };
+ constexpr const auto check = [](auto & ptr) { EXPECT_THROW(itk::Deref(ptr), itk::DerefError); }; I'm no C++ expert, but is it right to have both Also, I was surprised to see the 2nd const removed. And this removal generates one of the many compiler errors. Still, this is looking like it'll be great! |
I got some fixits that do not compile, e.g.:
is changed into
|
The reason is that when inserting, I add a
Eh... yeah. The range I passed to the
Thanks, and thank you very much for trying it out. My current setup is limited in what I can test outside the LLVM codebase. |
Another interesting case: static int testToWindowsExtendedPath()
{
#ifdef _WIN32
// lots of real, non constant work
return ret;
#else
return 0;
#endif
} For some platforms this can be constexpr; for other platforms (Windows), it cannot. It was transformed to |
I've fixed:
Not yet addressed:
static int testToWindowsExtendedPath()
{
#ifdef _WIN32
// lots of real, non constant work
return ret;
#else
return 0;
#endif
} This will not be possible to detect. The check works once the AST is formed, so after the preprocessor. It can't analyze multiple preprocessor branches at the same time. I also ran on all of llvm-project/llvm/ without issue |
I tested 6040ba1 and found this fixit that does not compile: Input namespace
{
struct N
{
[[maybe_unused]] friend void operator<<(int&, const N&) {}
};
} clang-tidy -checks="-*,*constexpr*" -export-fixes fixes.yaml test.cpp -- --std=c++23 Result namespace
{
struct N
{
constexpr [[maybe_unused]] friend void operator<<(int&, const N&) {}
};
} |
The insertion location was broken due to the insertion point being possibly in front of attributes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gave this a very rought look, so only a couple of general questions/suggestions.
Could we do something with methods that repeat in some of the visitors, e.g. bool TraverseType(QualType QT)
and others. Maybe macros..? I can't think of a better idea right now.
Could we break this PR in multiple ones, e.g. first with support of CXX11, then CXX14 and so on? 1000-line .cpp
file is hard to review. Also, with a PR for each standart we will understand better what changed and what new constexpr cases appear.
Fixed multiple vars declared in one statement |
I will try to think of something
That's a valid point. I'll keep this as is for a while longer without the expectation that people will review it, so that I can resolve a few issues. I also want to fix my setup to be able to run on external code bases. Then I will split it up into {[11],[14, 17],[20],[23, 26]} for 4 reviews. I'm not splitting per version since the two pairs of versions are quite similar to each other. The PRs will probably be serialized and not stacked, because we all know how nice GitHubs PR stacking works. |
Please rebase from |
BTW does this support C23? That would be nice to have too. |
I totally forgot to add that :) I'll take a look at the rules for it, but might only add it later. It looks like it's quite simple: only scalar variables with a few rules around what's possible |
I've just tried VTK again, and seems there's only one compiler error now: - operator bool() const noexcept { return this->InT >= 0.0 && this->OutT >= 0.0; }
+ operator constexpr bool() const noexcept { return this->InT >= 0.0 && this->OutT >= 0.0; }
|
I'll have very little access to my machine this month, but I'll move this forward in October (maybe with a bit before that) and also split the PR into smaller ones. |
I've cleaned things up and separated-out the C++-11-only rule-set into a new PR for a more incremental review and merge here: #162741
|
Did you have a chance to add C23 support? (And by extension Objective-C, which is what most of my code is :)) |
@@ -0,0 +1,47 @@ | |||
//===--- UseUseConstexprCheck.h - clang-tidy --------------------*- C++ -*-===// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//===--- UseUseConstexprCheck.h - clang-tidy --------------------*- C++ -*-===// | |
//===----------------------------------------------------------------------===// |
@@ -0,0 +1,1038 @@ | |||
//===--- UseConstexprCheck.cpp - clang-tidy--------------------------------===// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//===--- UseConstexprCheck.cpp - clang-tidy--------------------------------===// | |
//===----------------------------------------------------------------------===// |
/// Finds functions and variables that can be declared 'constexpr'. | ||
/// | ||
/// For the user-facing documentation see: | ||
/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-constexpr.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-constexpr.html | |
/// https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-constexpr.html |
- New :doc:`modernize-use-constexpr | ||
<clang-tidy/checks/modernize/use-constexpr>` check. | ||
|
||
Finds functions and variables that can be declared `constexpr`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finds functions and variables that can be declared `constexpr`. | |
Finds functions and variables that can be declared ``constexpr``. |
|
||
The string to use with C++23 to specify a function-local variable as | ||
``static constexpr``, for example, a macro. Default is `static constexpr` | ||
(concatenating `static` with the `ConstexprString` option). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(concatenating `static` with the `ConstexprString` option). | |
(concatenating `static` with the :option:`ConstexprString`). |
I have not yet added C23 support, but what I had read-up on, it seemed to be straight-forward to do.
o.O :) I got no clue about Objective-C |
@EugeneZelenko I'll apply your review comments in the new PR |
Well, it's just a superset of C, so hopefully as long as you support C23, Obj-C should also 'just work' unless you do something to turn it off. |
PR will be split for easier review
This check finds all functions and variables that can be declared as
constexpr
, using the specified standard version to check if therequirements are met.
Fixes #115622